fix(service-automation): evaluation refuses a malformed condition shape with registration's own refusal - #16438
Conversation
…pe with registration's own refusal `evaluateCondition` derived `exprStr` with a `typeof` guard covering only the bare-string arm, so an envelope whose `source` was present and not a string became that value and `.trim()` threw a bare `TypeError` naming no flow, no node and no expression. Two sibling arms shared the same unguarded read: a value that is neither text nor envelope-shaped read as an EMPTY condition and answered a silent `false` — on the same key a start node's trigger gate is read from — and a malformed envelope under a non-predicate dialect answered `false` one statement earlier still, at the dialect check. #15662 closed this reject set at the producer; the evaluator was left disagreeing with it in a different vocabulary. Per the maintainer's ruling (decision batch #57, option A) evaluation now calls `structuralConditionRefusal` — the SAME constructor `registerFlow` calls, not a second hand-written envelope that could drift — as the method's first statement, above the dialect check so all three arms are covered. Controls pinned alongside: bare CEL text and both envelope spellings still evaluate; an `ast`-only envelope still answers `false` (that population is #15430/#15807's); a well-formed `cron` envelope still answers `false` rather than being refused; absent/`null`/empty/whitespace conditions are still "not authored"; and a malformed STRING still earns the brace trap or the §1c CEL fault, never the shape refusal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 2 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f3f8b6dfca071cbe465a822edf7536e84591ab55 && git checkout f3f8b6dfca071cbe465a822edf7536e84591ab55
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c14ebfeb29d8d3bc0ae9b148132a78b86dec94bc 59d78598201485d8a5f545cf117011ed08118f1e && git checkout -B drift-repro c14ebfeb29d8d3bc0ae9b148132a78b86dec94bc && git merge --no-ff 59d78598201485d8a5f545cf117011ed08118f1e
node scripts/docs-audit/affected-docs.mjs --json c14ebfeb29d8d3bc0ae9b148132a78b86dec94bc
|
github-actions
Bot
added
size/m
documentation
2 participants
Fixes #16038
Implements the maintainer's ruling of 2026-09-06 (decision batch #57, comment 5559820263), option A:
evaluateConditionrefuses a malformed condition shape with the sameSTRUCTURAL_CONDITION_SHAPE_REFUSALregistration already raises, so evaluation and registration share one refusal.Verified at
59d785982— the final head, and the commit every number below was measured on.The enumeration came first, and it found two arms the reproduction does not name
The ruling's first execution note is to enumerate every sibling site before patching the reported one. There is exactly one unguarded read in
evaluateCondition— theexprStrderivation — but it has three distinct measured failure arms, and a fix written at the reported line reaches only two of them:3e7ef9c23, drivenTypeErrorsource:{ source: 1 },{ dialect: 'cel', source: 1 },{ dialect: 'cel', source: {} },{ dialect: 'template', source: 1 }TypeError: exprStr.trim is not a function— naming no flow, no node, no expressionfalse42,true,['a'],{},{ dialect: 'cel' }falseoff the empty-source arm, nothing loggedfalse, one statement earlier{ dialect: 'cron', source: 1 }falseat the dialect pre-check, never reaching the derivationArm C is why the guard is the method's first statement rather than a patch at
exprStr. All three are refused bystructuralConditionRefusalat registration today, so all three are the ruling's population.Every helper the method delegates to —
templateHoles,celScope,refuseUnresolvedTemplateHole,refuseUnresolvedCelOperand,compareValues— is handedexprStrand nothing else, so one guard above the derivation closes the whole delegation tree.The sibling value path is NOT a site — measured, not assumed
evaluateValueEnvelopealready derives its verdict fromvalueEnvelopeRefusals, the same callregisterFlowmakes (#15137). Driven before the fix,{ source: 1 },{ dialect: 'cel', source: 1 },{ dialect: 'cel', source: {} },{ ast, source: 1 },{ dialect: 'cel' },42,['a']and{}each threw an attributed error leading withASSIGNMENT_VALUE_ENVELOPE_REFUSALor a located CEL fault. Zero rawTypeErrors. It is already this shape one door over, with its own shared constructor. Nothing moved there.One residual defect found there is deliberately not fixed here and is filed separately:
evaluateValueEnvelope(null)/(undefined)throwsTypeError: Cannot read properties of null (reading 'source'). It is a different refusal vocabulary the ruling does not name, and it is unreachable from the only production call site.No caller depended on the
TypeErrorRepo-wide, every occurrence of
is not a functionon this path is prose recording the pre-fix symptom — never an assertion, never acatchthat branches.structural-condition-shape.test.tsasserts the opposite direction already. The two engine-internal callers (the start gate atengine.tsand the edge gate) call it bare, so a throw propagates toexecute()'s catch and is recorded as a loud flow failure — ADR-0032 §1c's prescribed handling. The screen-field caller passesString(...)and cannot reach the site. The ruling lands as written.logic-nodes.ts:73—cond.expressionis not guaranteed a stringconditionsis an uncheckedas Arraycast taken offnode.config, an openz.recordthatDecisionConditionSchemais never parsed against. The only thing making it a string is registration'spredicateSlotRefusalgate (#15572) — a producer-side gate. Measured: registering such a flow throws today, so authored metadata cannot reach it; a flow stored before that gate landed, or a direct caller of this public method on an exported class, still can. That asymmetry is exactly what this PR closes.One constructor, not two envelopes
The guard calls
structuralConditionRefusal— the identical importregisterFlowalready uses at the same file. No second hand-written envelope was introduced, and no file underpackages/specis touched: the constructor was already exported and already imported byengine.ts.Tests — red before green, per site
structural-condition-shape.test.tsgains 18 tests: one per refused shape across all three arms, three property tests, and five CONTROL tests. The prediction was written before the run and matched exactly.Tests 13 failed | 18 passed (31)— the 10 per-arm rows plus the 3 property tests red; the 5 new controls green alongside the 13 pre-existing service-automation:evaluateConditionanswers a silentfalsefor a non-string predicate, and a non-stringconfig.conditionregisters clean #15662 tests.Tests 31 passed (31).The strongest of the three property tests walks one shared population through both doors and asserts both refuse with the same published sentence — the machine-checkable form of "the two reject sets are one set". Two envelopes that drifted apart would fail there while every per-site row stayed green.
Ablation, from the committed state, with both legs proven on disk:
1to0, sentinel0to1, blob213c3d65to804490eb; result13 failed | 18 passed— the same 13, so the mutation demonstrably reached the subject. No rebuild is involved: the suite imports./engine.jsrelative source, not a packageexportsentry.git checkout HEAD -- ABSOLUTE_PATH(never the bare form, which takes from a polluted index), then blob equals the HEAD blob213c3d65with both hashes non-empty, sentinel back to0, guard-call back to1, andgit diff HEADzero bytes.Controls pinned, so the guard cannot over-reach
Bare CEL text and both envelope spellings still evaluate; an
ast-only envelope still answersfalse(that population is #15430/#15807's, deliberately untouched); a well-formedcronenvelope still answersfalserather than being refused; absent,null, empty and whitespace-only conditions are still "not authored"; and a malformed string still earns the brace trap or the §1c CEL fault, never the shape refusal.Changeset
Included,
minor,@objectstack/service-automationonly. Required because this is shipped runtime source in a published package and the change is user-observable: a stored flow carrying a refused shape previously ran (silentlyfalse, or faulted unattributed) and now fails loudly.minorrather thanpatchmatches the sibling #15662 bump for the same class — a previously-accepted population is now refused.packages/specandpackages/lintare unversioned here because neither is modified.Verification
structural-condition-shape.test.ts— 31/31.@objectstack/service-automationfull suite —Test Files 124 passed (124),Tests 1461 passed (1461).typecheckandcheck:test-typecheck— green. The latter compiles the test layer undertsconfig.test.json, so the new tests are actually type-checked rather than excluded.connector-mcp23/23,trigger-record-change101/101,trigger-schedule57/57,plugin-approvals690/690. An initialconnector-mcpfailure was an unbuilt-distcollection error, not a finding; it passes once the prerequisite is met.node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, plus the four artifact-roster families whose roster sits under a directory this diff touches: 60/60 green. Three initially returnedPREREQUISITE NOT MET(unbuilt monorepodist) — recorded as not-measured, then re-run to a real reading afterturbo run buildover the packages closure, exactly aslint.ymldoes.Local scope was the affected package plus its consumers and the derived families; the repo-wide scans remain CI's run.
Generated by Claude Code